Automated Test: fix/handle-collective-multiple-host-destinations #367
Conversation
📝 WalkthroughWalkthroughThis PR refactors destination calendar handling across the codebase to use arrays instead of single values, updating type definitions, calendar service implementations, event managers, and booking workflows. It also adds credential ID parameters to calendar operations and adjusts email template rendering for the new structure. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
packages/app-store/office365calendar/lib/CalendarService.ts (1)
94-109:⚠️ Potential issue | 🟠 MajorApply the same dynamic calendar selection logic to
updateEventthatcreateEventuses.The
updateEventmethod receives theeventparameter containingdestinationCalendarbut uses the hardcoded path/me/calendar/events/${uid}, whilecreateEventcorrectly selects the calendar endpoint based onmainHostDestinationCalendar?.externalId. If an event was created in a non-default calendar, updating it via the hardcoded default calendar path will fail or update the wrong event. ExtractmainHostDestinationCalendarfromevent.destinationCalendarand construct the path as:const eventsUrl = mainHostDestinationCalendar?.externalId ? `/me/calendars/${mainHostDestinationCalendar?.externalId}/events/${uid}` : `/me/calendar/events/${uid}`;packages/lib/CalendarService.ts (1)
125-125:⚠️ Potential issue | 🔴 CriticalInterface compliance:
createEventsignature mismatch.The
Calendarinterface requirescreateEvent(event: CalendarEvent, credentialId: number), but this implementation uses onlycreateEvent(event: CalendarEvent). This will cause a TypeScript compilation error.🐛 Proposed fix to match interface signature
- async createEvent(event: CalendarEvent): Promise<NewCalendarEventType> { + async createEvent(event: CalendarEvent, credentialId: number): Promise<NewCalendarEventType> {Note: If
credentialIdis not needed in this implementation, it can be prefixed with underscore:_credentialId: numberpackages/features/bookings/lib/handleNewBooking.ts (1)
987-1079:⚠️ Potential issue | 🟠 MajorCollective team destination calendars can be dropped
When neither
eventType.destinationCalendarnororganizerUser.destinationCalendarexist,evt.destinationCalendaris set tonull, and the optional-chained push at Line 1078 skipsteamDestinationCalendars. For COLLECTIVE, this silently omits team members’ calendars.🔧 Suggested fix
- destinationCalendar: eventType.destinationCalendar - ? [eventType.destinationCalendar] - : organizerUser.destinationCalendar - ? [organizerUser.destinationCalendar] - : null, + destinationCalendar: eventType.destinationCalendar + ? [eventType.destinationCalendar] + : organizerUser.destinationCalendar + ? [organizerUser.destinationCalendar] + : [], @@ - if (isTeamEventType && eventType.schedulingType === "COLLECTIVE") { - evt.destinationCalendar?.push(...teamDestinationCalendars); - } + if (isTeamEventType && eventType.schedulingType === "COLLECTIVE") { + evt.destinationCalendar = [...(evt.destinationCalendar ?? []), ...teamDestinationCalendars]; + }packages/app-store/googlecalendar/lib/CalendarService.ts (2)
145-178:⚠️ Potential issue | 🟡 MinorPatch receives undefined while insert falls back to "primary"—use consistent calendarId for both calls
Line 154 uses
selectedCalendar || "primary"for the insert call, but line 168 passesselectedCalendardirectly to patch without the fallback. When no destination calendar matches the credentialId, the insert succeeds with the "primary" calendar, but the patch receivesundefinedand may fail silently. Extract the fallback once and reuse it for both operations.🔧 Suggested fix
const selectedCalendar = calEventRaw.destinationCalendar?.find( (cal) => cal.credentialId === credentialId )?.externalId; + const calendarId = selectedCalendar || "primary"; calendar.events.insert( { auth: myGoogleAuth, - calendarId: selectedCalendar || "primary", + calendarId, requestBody: payload, conferenceDataVersion: 1, sendUpdates: "none", }, function (error, event) { if (event && event.data.id && event.data.hangoutLink) { calendar.events.patch({ // Update the same event but this time we know the hangout link - calendarId: selectedCalendar, + calendarId, auth: myGoogleAuth, eventId: event.data.id || "", requestBody: {
198-230:⚠️ Potential issue | 🟠 Major
updateEventcan send an undefined calendarIdWhen
externalCalendarIdis falsy, the fallback logic comparescal.externalId === externalCalendarId(undefined), which never matches, leavingselectedCalendarundefined. This causes the API call to fail. Use the resolved destination calendar instead.🔧 Suggested fix
- const selectedCalendar = externalCalendarId - ? externalCalendarId - : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId; + const selectedCalendar = + externalCalendarId || + mainHostDestinationCalendar?.externalId || + event.destinationCalendar?.[0]?.externalId || + "primary";
🤖 Fix all issues with AI agents
In `@packages/app-store/googlecalendar/lib/CalendarService.ts`:
- Around line 315-318: The fallback for computing calendarId is incorrect:
change the logic in the delete path (where calendarId is computed using
externalCalendarId and event.destinationCalendar) so that when
externalCalendarId is null/undefined you use the first destination calendar's
externalId if present, otherwise fall back to "primary"; update the code that
computes calendarId (reference externalCalendarId, calendarId, and
event.destinationCalendar in the deleteEvent flow) to implement this new
fallback order.
In `@packages/app-store/larkcalendar/lib/CalendarService.ts`:
- Around line 125-131: The createEvent implementation in CalendarService.ts must
match the Calendar interface: change the method signature of createEvent to
include the second parameter credentialId: number (i.e., createEvent(event:
CalendarEvent, credentialId: number)), update any internal references to accept
and use credentialId as needed, and propagate this signature change to all
callers of CalendarService.createEvent (and other calendar service
implementations like Office365/Exchange/Hubspot/Salesforce) so the type contract
in packages/types/Calendar.d.ts is satisfied.
In `@packages/core/EventManager.ts`:
- Around line 116-120: Guard against missing destination calendars by ensuring
mainHostDestinationCalendar exists before reading its .integration: in
EventManager.ts replace the current conditional that checks evt.location ===
MeetLocationType && mainHostDestinationCalendar.integration !==
"google_calendar" with a check that first verifies mainHostDestinationCalendar
is defined (e.g., mainHostDestinationCalendar &&
mainHostDestinationCalendar.integration !== "google_calendar") so you don't
access .integration on undefined before setting evt.location to
"integrations:daily".
- Around line 511-543: The code may call updateEvent(credential, ...) with
credential === undefined when credential is not found in calendarCredentials or
credentialFromDB; instead, after the DB lookup in the block guarded by
reference.credentialId, check if credential is still undefined and if so push a
per-reference failure into result (marking this specific bookingRefUid/reference
as failed with an error like "missing credential") and skip calling updateEvent
for that reference, otherwise call result.push(updateEvent(...)) as before;
reference symbols: reference.credentialId, calendarCredentials,
credentialFromDB, updateEvent, bookingRefUid, calenderExternalId.
In `@packages/features/bookings/lib/handleCancelBooking.ts`:
- Around line 251-255: Compute a single normalized value for destinationCalendar
(e.g., get dest = bookingToDelete?.destinationCalendar ??
bookingToDelete?.user.destinationCalendar ?? []) and then set
destinationCalendar = Array.isArray(dest) ? dest : [dest] so you never produce
nested arrays; replace the existing inline ternary that wraps values in [] with
this normalized value and reuse it in both payloads (also update the other
occurrence referenced around lines 528-532).
- Around line 418-484: In handleCancelBooking: bookingCalendarReference
currently leads to only the first _calendar ref being used per updated booking
and the recurring-deletion logic runs for each bookingCalendarReference (causing
duplicate API calls); change the per-updated-booking lookup from find(...) to
filter(...) so you iterate all calendar references for each updated booking (use
bookingRef(s) instead of a single bookingRef), resolve calendar credentials per
reference (use reference.credentialId to fetch the specific credential via
prisma if not present in bookingToDelete.user.credentials), and move/guard the
recurring branch (the block that maps
bookingToDelete.user.credentials/getCalendar and iterates updatedBookings) so it
executes once for the recurring case rather than once per
bookingCalendarReference; keep using getCalendar and calendar.deleteEvent(uid,
evt, externalCalendarId) but collect promises into apiDeletes without
duplicating calls.
In `@packages/types/Calendar.d.ts`:
- Line 221: Update each CalendarService implementation to match the new
interface: change the createEvent signature to accept (event: CalendarEvent,
credentialId: number): Promise<NewCalendarEventType>, update any internal calls
and parameter passing to forward credentialId to downstream clients or API
helpers, and adjust any mock/tests that call createEvent; specifically edit the
createEvent method in the listed implementations (e.g., CalendarService in
packages/lib, packages/app-store/*/lib) so they accept the credentialId
parameter and use it where credentials/authorization are selected or passed
along.
| const calendarId = externalCalendarId | ||
| ? externalCalendarId | ||
| : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/app-store/googlecalendar/lib && wc -l CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 112
🏁 Script executed:
cd packages/app-store/googlecalendar/lib && sed -n '300,330p' CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1119
🏁 Script executed:
cd packages/app-store/googlecalendar/lib && grep -n "deleteEvent" CalendarService.ts | head -20Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 201
🏁 Script executed:
cd packages/app-store/googlecalendar && find . -name "*.ts" -type f | xargs grep -l "interface CalendarEvent\|type CalendarEvent" | head -5Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
🏁 Script executed:
cd packages && rg "interface CalendarEvent|type CalendarEvent" -A 10Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1896
🏁 Script executed:
cd packages/app-store/googlecalendar/lib && sed -n '306,340p' CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1428
🏁 Script executed:
cd packages && rg "interface CalendarEvent" -A 50 types/Calendar.d.ts | head -80Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1752
🏁 Script executed:
cd packages/app-store/googlecalendar/lib && grep -n "destinationCalendar" CalendarService.ts | head -20Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 719
🏁 Script executed:
cd packages && rg "destinationCalendar" -B 2 -A 2 types/Calendar.d.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 293
🏁 Script executed:
cd packages && rg "interface DestinationCalendar|type DestinationCalendar" -A 10 types/Calendar.d.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
🏁 Script executed:
cd packages/app-store/googlecalendar/lib && sed -n '250,270p' CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 755
🏁 Script executed:
cd packages && rg "DestinationCalendar" types/Calendar.d.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 241
🏁 Script executed:
cd packages && rg "export.*DestinationCalendar" -A 10Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 2443
🏁 Script executed:
cd packages && find . -name "*.d.ts" -type f | xargs grep -l "DestinationCalendar" | head -5Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 111
🏁 Script executed:
cd packages && rg "model DestinationCalendar" -A 15Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1030
🏁 Script executed:
cd packages && rg "SelectedCalendar" types/Calendar.d.ts -B 2 -A 2Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 409
🏁 Script executed:
cd packages/app-store/googlecalendar/lib && sed -n '240,280p' CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1431
🏁 Script executed:
cd packages/app-store/googlecalendar/lib && sed -n '230,260p' CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 971
🏁 Script executed:
cd packages/app-store/googlecalendar/lib && grep -B 5 "async updateEvent" CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 220
🏁 Script executed:
cd packages && rg "deleteEvent\|updateEvent" --type ts -A 3 packages/app-store/googlecalendar/lib/CalendarService.ts | head -60Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 186
🏁 Script executed:
cd packages/app-store/googlecalendar/lib && rg "async.*Event" CalendarService.ts | head -20Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 399
🏁 Script executed:
cd packages/app-store/googlecalendar/lib && sed -n '140,180p' CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1519
🏁 Script executed:
cd packages/app-store/googlecalendar/lib && grep -n "async createEvent\|async updateEvent\|async deleteEvent" CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 410
🏁 Script executed:
cd packages/app-store/googlecalendar/lib && sed -n '87,160p' CalendarService.ts | head -60Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 2271
🏁 Script executed:
cd packages/app-store/googlecalendar/lib && sed -n '306,340p' CalendarService.tsRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1428
Fix deleteEvent fallback logic when externalCalendarId is missing
The fallback logic on line 315-318 is broken. When externalCalendarId is undefined/null, the .find() searches for cal.externalId === undefined, which never matches any calendar. This causes calendarId to become undefined, forcing deletion to always target the "primary" calendar regardless of where the event was created. Since 404 errors are silently swallowed (line 339), deletion silently fails when the event is on another calendar.
Use the first destination calendar or primary directly:
🔧 Suggested fix
- const calendarId = externalCalendarId
- ? externalCalendarId
- : event.destinationCalendar?.find((cal) => cal.externalId === externalCalendarId)?.externalId;
+ const calendarId =
+ externalCalendarId || event.destinationCalendar?.[0]?.externalId || defaultCalendarId;
calendar.events.delete(
{
auth: myGoogleAuth,
- calendarId: calendarId ? calendarId : defaultCalendarId,
+ calendarId,🤖 Prompt for AI Agents
In `@packages/app-store/googlecalendar/lib/CalendarService.ts` around lines 315 -
318, The fallback for computing calendarId is incorrect: change the logic in the
delete path (where calendarId is computed using externalCalendarId and
event.destinationCalendar) so that when externalCalendarId is null/undefined you
use the first destination calendar's externalId if present, otherwise fall back
to "primary"; update the code that computes calendarId (reference
externalCalendarId, calendarId, and event.destinationCalendar in the deleteEvent
flow) to implement this new fallback order.
| async createEvent(event: CalendarEvent): Promise<NewCalendarEventType> { | ||
| let eventId = ""; | ||
| let eventRespData; | ||
| const calendarId = event.destinationCalendar?.externalId; | ||
| const [mainHostDestinationCalendar] = event.destinationCalendar ?? []; | ||
| const calendarId = mainHostDestinationCalendar?.externalId; | ||
| if (!calendarId) { | ||
| throw new Error("no calendar id"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the Calendar interface definition
fd "\.ts$" packages/app-store | xargs rg "interface Calendar" -A 20Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1562
🏁 Script executed:
# Check the actual LarkCalendarService createEvent signature
cat packages/app-store/larkcalendar/lib/CalendarService.ts | head -150Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 5129
🏁 Script executed:
# Find all calendar services
fd -type d "packages/app-store/*/lib" | grep -E "calendar|outlook|google" | head -10Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 260
🏁 Script executed:
# Find the Calendar interface definition in types
fd -type f "Calendar.ts" packages/Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 260
🏁 Script executed:
# Search for interface Calendar in the codebase
rg "interface Calendar" --type ts -A 15Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 3906
🏁 Script executed:
# Find all calendar services and their createEvent signatures
find packages/app-store -name "CalendarService.ts" -o -name "calendar.ts" | head -20Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 919
🏁 Script executed:
# Check other calendar services
ls -la packages/app-store | grep -E "google|outlook|caldav|ical"Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 284
🏁 Script executed:
# Check Google Calendar Service createEvent signature
head -200 packages/app-store/googlecalendar/lib/CalendarService.ts | grep -A 5 "createEvent"Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 396
🏁 Script executed:
# Check Outlook Calendar Service createEvent signature
head -200 packages/app-store/office365calendar/lib/CalendarService.ts | grep -A 5 "createEvent"Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 422
🏁 Script executed:
# Check CalDAV Calendar Service createEvent signature
head -200 packages/app-store/caldavcalendar/lib/CalendarService.ts | grep -A 5 "createEvent"Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
🏁 Script executed:
# Count how many calendar services have the credentialId parameter
rg "async createEvent\(event: CalendarEvent, credentialId: number\)" packages/app-storeRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 72
🏁 Script executed:
# Count how many calendar services still have the old signature
rg "async createEvent\(event: CalendarEvent\):" packages/app-storeRepository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 1649
🏁 Script executed:
# Check if tsconfig enforces strict type checking
cat tsconfig.json | head -50Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 135
🏁 Script executed:
# Check if there's a build or type-check script
cat package.json | grep -A 5 '"scripts"'Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 353
🏁 Script executed:
# Look for any type errors or TODO comments about the credentialId migration
rg "credentialId" packages/app-store --type ts -B 2 -A 2 | head -100Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 7274
Interface compliance: createEvent signature mismatch with Calendar interface.
The Calendar interface (packages/types/Calendar.d.ts) requires createEvent(event: CalendarEvent, credentialId: number), but this implementation uses createEvent(event: CalendarEvent) without the credentialId parameter. This violates the interface contract and will cause a TypeScript type error.
Note: This issue affects multiple calendar services (Office 365, Exchange, Hubspot, Salesforce, and others) — only the Google Calendar service has been updated so far. Consider whether this is part of a broader migration effort.
🤖 Prompt for AI Agents
In `@packages/app-store/larkcalendar/lib/CalendarService.ts` around lines 125 -
131, The createEvent implementation in CalendarService.ts must match the
Calendar interface: change the method signature of createEvent to include the
second parameter credentialId: number (i.e., createEvent(event: CalendarEvent,
credentialId: number)), update any internal references to accept and use
credentialId as needed, and propagate this signature change to all callers of
CalendarService.createEvent (and other calendar service implementations like
Office365/Exchange/Hubspot/Salesforce) so the type contract in
packages/types/Calendar.d.ts is satisfied.
| // Fallback to Cal Video if Google Meet is selected w/o a Google Cal | ||
| if (evt.location === MeetLocationType && evt.destinationCalendar?.integration !== "google_calendar") { | ||
| // @NOTE: destinationCalendar it's an array now so as a fallback we will only check the first one | ||
| const [mainHostDestinationCalendar] = evt.destinationCalendar ?? []; | ||
| if (evt.location === MeetLocationType && mainHostDestinationCalendar.integration !== "google_calendar") { | ||
| evt["location"] = "integrations:daily"; |
There was a problem hiding this comment.
Guard against undefined destination calendar in Meet fallback
mainHostDestinationCalendar can be undefined when no destination calendars are set; accessing .integration will throw. Line 119 should tolerate missing values.
🔧 Suggested fix
- const [mainHostDestinationCalendar] = evt.destinationCalendar ?? [];
- if (evt.location === MeetLocationType && mainHostDestinationCalendar.integration !== "google_calendar") {
+ const [mainHostDestinationCalendar] = evt.destinationCalendar ?? [];
+ if (
+ evt.location === MeetLocationType &&
+ mainHostDestinationCalendar?.integration !== "google_calendar"
+ ) {
evt["location"] = "integrations:daily";
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Fallback to Cal Video if Google Meet is selected w/o a Google Cal | |
| if (evt.location === MeetLocationType && evt.destinationCalendar?.integration !== "google_calendar") { | |
| // @NOTE: destinationCalendar it's an array now so as a fallback we will only check the first one | |
| const [mainHostDestinationCalendar] = evt.destinationCalendar ?? []; | |
| if (evt.location === MeetLocationType && mainHostDestinationCalendar.integration !== "google_calendar") { | |
| evt["location"] = "integrations:daily"; | |
| // Fallback to Cal Video if Google Meet is selected w/o a Google Cal | |
| // `@NOTE`: destinationCalendar it's an array now so as a fallback we will only check the first one | |
| const [mainHostDestinationCalendar] = evt.destinationCalendar ?? []; | |
| if ( | |
| evt.location === MeetLocationType && | |
| mainHostDestinationCalendar?.integration !== "google_calendar" | |
| ) { | |
| evt["location"] = "integrations:daily"; |
🤖 Prompt for AI Agents
In `@packages/core/EventManager.ts` around lines 116 - 120, Guard against missing
destination calendars by ensuring mainHostDestinationCalendar exists before
reading its .integration: in EventManager.ts replace the current conditional
that checks evt.location === MeetLocationType &&
mainHostDestinationCalendar.integration !== "google_calendar" with a check that
first verifies mainHostDestinationCalendar is defined (e.g.,
mainHostDestinationCalendar && mainHostDestinationCalendar.integration !==
"google_calendar") so you don't access .integration on undefined before setting
evt.location to "integrations:daily".
| if (reference.credentialId) { | ||
| credential = this.calendarCredentials.filter( | ||
| (credential) => credential.id === reference?.credentialId | ||
| )[0]; | ||
| if (!credential) { | ||
| // Fetch credential from DB | ||
| const credentialFromDB = await prisma.credential.findUnique({ | ||
| include: { | ||
| app: { | ||
| select: { | ||
| slug: true, | ||
| }, | ||
| }, | ||
| }, | ||
| where: { | ||
| id: reference.credentialId, | ||
| }, | ||
| }); | ||
| if (credentialFromDB && credentialFromDB.app?.slug) { | ||
| credential = { | ||
| appName: credentialFromDB?.app.slug ?? "", | ||
| id: credentialFromDB.id, | ||
| type: credentialFromDB.type, | ||
| key: credentialFromDB.key, | ||
| userId: credentialFromDB.userId, | ||
| teamId: credentialFromDB.teamId, | ||
| invalid: credentialFromDB.invalid, | ||
| appId: credentialFromDB.appId, | ||
| }; | ||
| } | ||
| } | ||
| result.push(updateEvent(credential, event, bookingRefUid, calenderExternalId)); | ||
| } else { |
There was a problem hiding this comment.
Handle missing credential before calling updateEvent
If the credential isn’t found in memory or DB, updateEvent(credential, …) receives undefined, which throws and forces the catch block to mark all references as failed. Emit a per-reference failure instead.
🔧 Suggested fix
if (reference.credentialId) {
credential = this.calendarCredentials.filter(
(credential) => credential.id === reference?.credentialId
)[0];
if (!credential) {
// Fetch credential from DB
const credentialFromDB = await prisma.credential.findUnique({
include: {
app: {
select: {
slug: true,
},
},
},
where: {
id: reference.credentialId,
},
});
if (credentialFromDB && credentialFromDB.app?.slug) {
credential = {
appName: credentialFromDB?.app.slug ?? "",
id: credentialFromDB.id,
type: credentialFromDB.type,
key: credentialFromDB.key,
userId: credentialFromDB.userId,
teamId: credentialFromDB.teamId,
invalid: credentialFromDB.invalid,
appId: credentialFromDB.appId,
};
}
}
- result.push(updateEvent(credential, event, bookingRefUid, calenderExternalId));
+ if (!credential) {
+ result.push(
+ Promise.resolve({
+ appName: "none",
+ type: reference.type,
+ success: false,
+ uid: "",
+ originalEvent: event,
+ credentialId: reference.credentialId ?? 0,
+ })
+ );
+ continue;
+ }
+ result.push(updateEvent(credential, event, bookingRefUid, calenderExternalId));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (reference.credentialId) { | |
| credential = this.calendarCredentials.filter( | |
| (credential) => credential.id === reference?.credentialId | |
| )[0]; | |
| if (!credential) { | |
| // Fetch credential from DB | |
| const credentialFromDB = await prisma.credential.findUnique({ | |
| include: { | |
| app: { | |
| select: { | |
| slug: true, | |
| }, | |
| }, | |
| }, | |
| where: { | |
| id: reference.credentialId, | |
| }, | |
| }); | |
| if (credentialFromDB && credentialFromDB.app?.slug) { | |
| credential = { | |
| appName: credentialFromDB?.app.slug ?? "", | |
| id: credentialFromDB.id, | |
| type: credentialFromDB.type, | |
| key: credentialFromDB.key, | |
| userId: credentialFromDB.userId, | |
| teamId: credentialFromDB.teamId, | |
| invalid: credentialFromDB.invalid, | |
| appId: credentialFromDB.appId, | |
| }; | |
| } | |
| } | |
| result.push(updateEvent(credential, event, bookingRefUid, calenderExternalId)); | |
| } else { | |
| if (reference.credentialId) { | |
| credential = this.calendarCredentials.filter( | |
| (credential) => credential.id === reference?.credentialId | |
| )[0]; | |
| if (!credential) { | |
| // Fetch credential from DB | |
| const credentialFromDB = await prisma.credential.findUnique({ | |
| include: { | |
| app: { | |
| select: { | |
| slug: true, | |
| }, | |
| }, | |
| }, | |
| where: { | |
| id: reference.credentialId, | |
| }, | |
| }); | |
| if (credentialFromDB && credentialFromDB.app?.slug) { | |
| credential = { | |
| appName: credentialFromDB?.app.slug ?? "", | |
| id: credentialFromDB.id, | |
| type: credentialFromDB.type, | |
| key: credentialFromDB.key, | |
| userId: credentialFromDB.userId, | |
| teamId: credentialFromDB.teamId, | |
| invalid: credentialFromDB.invalid, | |
| appId: credentialFromDB.appId, | |
| }; | |
| } | |
| } | |
| if (!credential) { | |
| result.push( | |
| Promise.resolve({ | |
| appName: "none", | |
| type: reference.type, | |
| success: false, | |
| uid: "", | |
| originalEvent: event, | |
| credentialId: reference.credentialId ?? 0, | |
| }) | |
| ); | |
| continue; | |
| } | |
| result.push(updateEvent(credential, event, bookingRefUid, calenderExternalId)); | |
| } else { |
🤖 Prompt for AI Agents
In `@packages/core/EventManager.ts` around lines 511 - 543, The code may call
updateEvent(credential, ...) with credential === undefined when credential is
not found in calendarCredentials or credentialFromDB; instead, after the DB
lookup in the block guarded by reference.credentialId, check if credential is
still undefined and if so push a per-reference failure into result (marking this
specific bookingRefUid/reference as failed with an error like "missing
credential") and skip calling updateEvent for that reference, otherwise call
result.push(updateEvent(...)) as before; reference symbols:
reference.credentialId, calendarCredentials, credentialFromDB, updateEvent,
bookingRefUid, calenderExternalId.
| destinationCalendar: bookingToDelete?.destinationCalendar | ||
| ? [bookingToDelete?.destinationCalendar] | ||
| : bookingToDelete?.user.destinationCalendar | ||
| ? [bookingToDelete?.user.destinationCalendar] | ||
| : [], |
There was a problem hiding this comment.
Normalize destinationCalendar without nesting arrays.
With array-based destination calendars, wrapping the value in [] will produce [[...]] (and [[]] for empty arrays), which breaks downstream expectations. Normalize with Array.isArray and reuse the normalized value for both payloads.
🛠️ Proposed fix
@@
- destinationCalendar: bookingToDelete?.destinationCalendar
- ? [bookingToDelete?.destinationCalendar]
- : bookingToDelete?.user.destinationCalendar
- ? [bookingToDelete?.user.destinationCalendar]
- : [],
+ destinationCalendar:
+ bookingToDelete?.destinationCalendar != null
+ ? Array.isArray(bookingToDelete.destinationCalendar)
+ ? bookingToDelete.destinationCalendar
+ : [bookingToDelete.destinationCalendar]
+ : bookingToDelete?.user.destinationCalendar != null
+ ? Array.isArray(bookingToDelete.user.destinationCalendar)
+ ? bookingToDelete.user.destinationCalendar
+ : [bookingToDelete.user.destinationCalendar]
+ : [],
@@
- destinationCalendar: bookingToDelete?.destinationCalendar
- ? [bookingToDelete?.destinationCalendar]
- : bookingToDelete?.user.destinationCalendar
- ? [bookingToDelete?.user.destinationCalendar]
- : [],
+ destinationCalendar:
+ bookingToDelete?.destinationCalendar != null
+ ? Array.isArray(bookingToDelete.destinationCalendar)
+ ? bookingToDelete.destinationCalendar
+ : [bookingToDelete.destinationCalendar]
+ : bookingToDelete?.user.destinationCalendar != null
+ ? Array.isArray(bookingToDelete.user.destinationCalendar)
+ ? bookingToDelete.user.destinationCalendar
+ : [bookingToDelete.user.destinationCalendar]
+ : [],Also applies to: 528-532
🤖 Prompt for AI Agents
In `@packages/features/bookings/lib/handleCancelBooking.ts` around lines 251 -
255, Compute a single normalized value for destinationCalendar (e.g., get dest =
bookingToDelete?.destinationCalendar ??
bookingToDelete?.user.destinationCalendar ?? []) and then set
destinationCalendar = Array.isArray(dest) ? dest : [dest] so you never produce
nested arrays; replace the existing inline ternary that wraps values in [] with
this normalized value and reuse it in both payloads (also update the other
occurrence referenced around lines 528-532).
| // Find the correct calendar credential under user credentials | ||
| const calendarCredential = bookingToDelete.user.credentials.find( | ||
| (credential) => credential.id === credentialId | ||
| ); | ||
| if (calendarCredential) { | ||
| const calendar = await getCalendar(calendarCredential); | ||
| if ( | ||
| bookingToDelete.eventType?.recurringEvent && | ||
| bookingToDelete.recurringEventId && | ||
| allRemainingBookings | ||
| ) { | ||
| const promises = bookingToDelete.user.credentials | ||
| .filter((credential) => credential.type.endsWith("_calendar")) | ||
| .map(async (credential) => { | ||
| const calendar = await getCalendar(credential); | ||
| for (const updBooking of updatedBookings) { | ||
| const bookingRef = updBooking.references.find((ref) => ref.type.includes("_calendar")); | ||
| if (bookingRef) { | ||
| const { uid, externalCalendarId } = bookingRef; | ||
| const deletedEvent = await calendar?.deleteEvent(uid, evt, externalCalendarId); | ||
| apiDeletes.push(deletedEvent); | ||
| if (bookingCalendarReference.length > 0) { | ||
| for (const reference of bookingCalendarReference) { | ||
| const { credentialId, uid, externalCalendarId } = reference; | ||
| // If the booking calendar reference contains a credentialId | ||
| if (credentialId) { | ||
| // Find the correct calendar credential under user credentials | ||
| let calendarCredential = bookingToDelete.user.credentials.find( | ||
| (credential) => credential.id === credentialId | ||
| ); | ||
| if (!calendarCredential) { | ||
| // get credential from DB | ||
| const foundCalendarCredential = await prisma.credential.findUnique({ | ||
| where: { | ||
| id: credentialId, | ||
| }, | ||
| }); | ||
| if (foundCalendarCredential) { | ||
| calendarCredential = foundCalendarCredential; | ||
| } | ||
| } | ||
| if (calendarCredential) { | ||
| const calendar = await getCalendar(calendarCredential); | ||
| if ( | ||
| bookingToDelete.eventType?.recurringEvent && | ||
| bookingToDelete.recurringEventId && | ||
| allRemainingBookings | ||
| ) { | ||
| const promises = bookingToDelete.user.credentials | ||
| .filter((credential) => credential.type.endsWith("_calendar")) | ||
| .map(async (credential) => { | ||
| const calendar = await getCalendar(credential); | ||
| for (const updBooking of updatedBookings) { | ||
| const bookingRef = updBooking.references.find((ref) => ref.type.includes("_calendar")); | ||
| if (bookingRef) { | ||
| const { uid, externalCalendarId } = bookingRef; | ||
| const deletedEvent = await calendar?.deleteEvent(uid, evt, externalCalendarId); | ||
| apiDeletes.push(deletedEvent); | ||
| } | ||
| } | ||
| }); | ||
| try { | ||
| await Promise.all(promises); | ||
| } catch (error) { | ||
| if (error instanceof Error) { | ||
| logger.error(error.message); | ||
| } | ||
| }); | ||
| try { | ||
| await Promise.all(promises); | ||
| } catch (error) { | ||
| if (error instanceof Error) { | ||
| logger.error(error.message); | ||
| } | ||
| } else { | ||
| apiDeletes.push(calendar?.deleteEvent(uid, evt, externalCalendarId) as Promise<unknown>); | ||
| } | ||
| } else { | ||
| } | ||
| } else { | ||
| // For bookings made before the refactor we go through the old behavior of running through each calendar credential | ||
| const calendarCredentials = bookingToDelete.user.credentials.filter((credential) => | ||
| credential.type.endsWith("_calendar") | ||
| ); | ||
| for (const credential of calendarCredentials) { | ||
| const calendar = await getCalendar(credential); | ||
| apiDeletes.push(calendar?.deleteEvent(uid, evt, externalCalendarId) as Promise<unknown>); | ||
| } | ||
| } | ||
| } else { | ||
| // For bookings made before the refactor we go through the old behaviour of running through each calendar credential | ||
| const calendarCredentials = bookingToDelete.user.credentials.filter((credential) => | ||
| credential.type.endsWith("_calendar") | ||
| ); | ||
| for (const credential of calendarCredentials) { | ||
| const calendar = await getCalendar(credential); | ||
| apiDeletes.push(calendar?.deleteEvent(uid, evt, externalCalendarId) as Promise<unknown>); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Recurring cancellations only delete the first calendar reference and can repeat per reference.
For multi-calendar bookings, find() removes only the first _calendar reference per updated booking, leaving other calendars untouched. Also, the recurring deletion runs once per bookingCalendarReference, which duplicates API calls. Iterate all calendar references and ensure the recurring branch runs only once.
🛠️ Proposed fix
- if (bookingCalendarReference.length > 0) {
- for (const reference of bookingCalendarReference) {
+ if (bookingCalendarReference.length > 0) {
+ let processedRecurring = false;
+ for (const reference of bookingCalendarReference) {
@@
- if (
- bookingToDelete.eventType?.recurringEvent &&
- bookingToDelete.recurringEventId &&
- allRemainingBookings
- ) {
+ if (
+ bookingToDelete.eventType?.recurringEvent &&
+ bookingToDelete.recurringEventId &&
+ allRemainingBookings
+ ) {
+ if (processedRecurring) continue;
+ processedRecurring = true;
const promises = bookingToDelete.user.credentials
.filter((credential) => credential.type.endsWith("_calendar"))
.map(async (credential) => {
const calendar = await getCalendar(credential);
for (const updBooking of updatedBookings) {
- const bookingRef = updBooking.references.find((ref) => ref.type.includes("_calendar"));
- if (bookingRef) {
- const { uid, externalCalendarId } = bookingRef;
- const deletedEvent = await calendar?.deleteEvent(uid, evt, externalCalendarId);
- apiDeletes.push(deletedEvent);
- }
+ const bookingRefs = updBooking.references.filter((ref) =>
+ ref.type.includes("_calendar")
+ );
+ for (const bookingRef of bookingRefs) {
+ const { uid, externalCalendarId } = bookingRef;
+ const deletedEvent = await calendar?.deleteEvent(uid, evt, externalCalendarId);
+ apiDeletes.push(deletedEvent);
+ }
}
});If each reference carries a credentialId, consider resolving the credential per reference instead of iterating all calendar credentials to avoid mismatched credential deletes.
🤖 Prompt for AI Agents
In `@packages/features/bookings/lib/handleCancelBooking.ts` around lines 418 -
484, In handleCancelBooking: bookingCalendarReference currently leads to only
the first _calendar ref being used per updated booking and the
recurring-deletion logic runs for each bookingCalendarReference (causing
duplicate API calls); change the per-updated-booking lookup from find(...) to
filter(...) so you iterate all calendar references for each updated booking (use
bookingRef(s) instead of a single bookingRef), resolve calendar credentials per
reference (use reference.credentialId to fetch the specific credential via
prisma if not present in bookingToDelete.user.credentials), and move/guard the
recurring branch (the block that maps
bookingToDelete.user.credentials/getCalendar and iterates updatedBookings) so it
executes once for the recurring case rather than once per
bookingCalendarReference; keep using getCalendar and calendar.deleteEvent(uid,
evt, externalCalendarId) but collect promises into apiDeletes without
duplicating calls.
| @@ -216,7 +218,7 @@ export interface IntegrationCalendar extends Ensure<Partial<SelectedCalendar>, " | |||
| } | |||
|
|
|||
| export interface Calendar { | |||
| createEvent(event: CalendarEvent): Promise<NewCalendarEventType>; | |||
| createEvent(event: CalendarEvent, credentialId: number): Promise<NewCalendarEventType>; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Run the provided verification script
rg -n 'async createEvent\(event: CalendarEvent\)' --type ts
# Also search for all createEvent implementations to get a complete picture
echo "=== All createEvent implementations ==="
rg -n 'createEvent\s*\(' --type ts -A 2Repository: admin-coderabbit/coderabbit-cal_dot_com-mirror
Length of output: 10412
Critical: 13 calendar implementations have not been updated with the new credentialId parameter.
The interface change requires all createEvent implementations to accept the credentialId parameter, but only the Google Calendar implementation has been updated. The following implementations still use the old signature and must be updated:
- packages/lib/CalendarService.ts
- packages/app-store/zoho-bigin/lib/CalendarService.ts
- packages/app-store/zohocrm/lib/CalendarService.ts
- packages/app-store/salesforce/lib/CalendarService.ts
- packages/app-store/sendgrid/lib/CalendarService.ts
- packages/app-store/office365calendar/lib/CalendarService.ts
- packages/app-store/larkcalendar/lib/CalendarService.ts
- packages/app-store/hubspot/lib/CalendarService.ts
- packages/app-store/exchangecalendar/lib/CalendarService.ts
- packages/app-store/exchange2016calendar/lib/CalendarService.ts
- packages/app-store/exchange2013calendar/lib/CalendarService.ts
- packages/app-store/closecom/lib/CalendarService.ts
- packages/app-store/basecamp3/lib/CalendarService.ts
🤖 Prompt for AI Agents
In `@packages/types/Calendar.d.ts` at line 221, Update each CalendarService
implementation to match the new interface: change the createEvent signature to
accept (event: CalendarEvent, credentialId: number):
Promise<NewCalendarEventType>, update any internal calls and parameter passing
to forward credentialId to downstream clients or API helpers, and adjust any
mock/tests that call createEvent; specifically edit the createEvent method in
the listed implementations (e.g., CalendarService in packages/lib,
packages/app-store/*/lib) so they accept the credentialId parameter and use it
where credentials/authorization are selected or passed along.
This pull request was automatically created by
@coderabbitai/e2e-reviewer.Batch created pull request.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor